Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add back default Encoder Callbacks for Keychron Q/V series #20307

Closed
wants to merge 12 commits into from

Conversation

adophoxia
Copy link
Contributor

Description

After #18713, the default encoder callbacks for the encoder variants of the Q2 were removed, and every Q and V board after that followed suit, which became problematic for users of QMK Configurator where after flashing the firmware from there, even with the encoder press working properly (KC_MUTE), rotation actions never work since as before, the callbacks were removed.

This PR is to add them back to rectify this issue to where even in the QMK Docs:

Those who are adding new keyboard support where encoders are enabled at the keyboard level should include basic encoder functionality at the keyboard level (<keyboard>.c) using the encoder_update_kb() function, that way it works for QMK Configuator users and exists in general.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@adophoxia
Copy link
Contributor Author

After some discussion with @drashna over in the QMK Discord, decided to move all of the encoder callbacks into the main <kb.c> file for each board instead of inside the <keyboard_variant.c> file to reduce boilerplate.

keyboards/keychron/q0/q0.c Outdated Show resolved Hide resolved
keyboards/keychron/q0/q0.c Outdated Show resolved Hide resolved
keyboards/keychron/q0/q0.c Outdated Show resolved Hide resolved
@adophoxia adophoxia requested a review from drashna April 3, 2023 06:43
@adophoxia adophoxia requested a review from waffle87 April 6, 2023 02:07
@vinorodrigues
Copy link
Contributor

With #20320 merged, is this even relevant? Literally does the same thing as the new quantum/encoder.c L79-L103

@adophoxia
Copy link
Contributor Author

After come careful consideration, I'm closing this PR due to the existence of #20320.

@adophoxia adophoxia closed this Apr 7, 2023
@adophoxia adophoxia deleted the keychron-encoder-c branch April 7, 2023 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants